Skip to content

Use cached npm info#451

Merged
andrewbranch merged 5 commits intomicrosoft:masterfrom
jablko:patch-48
Jun 9, 2022
Merged

Use cached npm info#451
andrewbranch merged 5 commits intomicrosoft:masterfrom
jablko:patch-48

Conversation

@jablko
Copy link
Contributor

@jablko jablko commented May 5, 2022

  • In generate-packages we know the npm info was cached in calculate-versions, so we don't consult the origin (npm registry) --- except in this one case, so fix that case (replace fetchAndCacheNpmInfo() with getNpmInfoFromCache()).
  • Add some static checks that we're not going to the origin, except in calculate-versions.
  • Don't bother writing the cache if there's no way we could have gone to the origin (correct but inconsequential optimization).

@jablko jablko force-pushed the patch-48 branch 2 times, most recently from 50afb55 to ffda973 Compare May 8, 2022 19:46
@jablko jablko force-pushed the patch-48 branch 3 times, most recently from 999ea2d to 74a5653 Compare May 16, 2022 15:09
@jablko
Copy link
Contributor Author

jablko commented May 16, 2022

I've rebased this on #456 and replaced UncachedNpmInfoClient/CachedNpmInfoClient with pacote, which the npm cli uses internally for fetching metadata from the registry.

The advantages over UncachedNpmInfoClient/CachedNpmInfoClient are caching and concurrency control:

  • Instead of hard coding whether the cache is fresh or stale (getNpmInfoFromCache() vs. fetchAndCacheNpmInfo()), using pacote.manifest() either way will use the cache if it's fresh and revalidate if not.
  • There's one case where we already check whether the cached typesPublisherContentHash is up to date, which I retained.
  • pacote handles concurrency internally via maxSockets, so wrapping it in nAtATime() is no longer needed.

@jablko jablko mentioned this pull request May 16, 2022
Merged
@jablko jablko force-pushed the patch-48 branch 5 times, most recently from 1e74144 to 2a8c8a3 Compare May 21, 2022 18:51
This was referenced May 22, 2022
Open
Open
@jablko jablko force-pushed the patch-48 branch 3 times, most recently from e850fcc to b7dd694 Compare May 30, 2022 16:13
@andrewbranch andrewbranch merged commit e246b08 into microsoft:master Jun 9, 2022
@andrewbranch andrewbranch mentioned this pull request Jun 9, 2022
Merged
andrewbranch added a commit that referenced this pull request Jun 9, 2022
@andrewbranch
Revert "Use cached npm info (#451)"
bb412f4 
This reverts commit e246b08.
andrewbranch added a commit that referenced this pull request Jun 9, 2022
@andrewbranch
Revert "Use cached npm info (#451)" (#480)
cbb1d4d 
* Revert "Use cached npm info (#451)"

This reverts commit e246b08.

* Fix package.json version
@jablko jablko mentioned this pull request Jun 16, 2022
Merged
@jablko jablko mentioned this pull request Jun 29, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sandersn sandersn Awaiting requested review from sandersn

@andrewbranch andrewbranch Awaiting requested review from andrewbranch

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jablko @andrewbranch